Badges to PR body #29702
Badges to PR body #29702
Conversation
|
🟢 |
|
Backported to |
naspirato
changed the title
This commit enhances the `test_validation.py` script by adding functionality to automatically determine the GitHub repository from the git remote URL. This improvement allows for better local testing by setting the `GITHUB_REPOSITORY` environment variable if it is not already defined. Key changes: - Introduced `find_github_repository` function to extract the repository name from the git remote URL. - Updated the main function to set `GITHUB_REPOSITORY` for local testing, improving usability and reducing manual setup requirements. - Enhanced documentation to clarify the automatic setting of `GITHUB_WORKSPACE` and `GITHUB_REPOSITORY` environment variables.
…te_pr_description.py`. The script now directly uses the PR body for validation, streamlining the process and improving usability.
This commit modifies the `validate_pr_description` workflow to introduce two new environment variables: `SHOW_RUN_TESTS_IN_PR` and `SHOW_BACKPORT_IN_PR`. These flags control the inclusion of test execution and backport tables in the PR body. The `ensure_tables_in_pr_body` function is updated to handle these flags, allowing for more flexible table generation based on user requirements. Key changes: - Replaced `SHOW_ADDITIONAL_INFO_IN_PR` with the new flags in the action configuration. - Updated `test_validation.py` to reflect the new environment variables and their usage. - Enhanced `validate_pr_description.py` to conditionally add tables based on the new flags, improving the clarity and usability of PR descriptions.
This commit enhances the `generate_backport_table` function in `validate_pr_description.py` to collect and sort unique branches from the input. The changes ensure that the backporting URL is generated for all unique branches, improving the accuracy and clarity of the backport table in PR descriptions. Key changes: - Updated logic to collect unique branches from multiple entries. - Sorted branches for consistent output. - Modified the backport button label for clarity.
|
⚪
🟢 |
|
⚪
🟢 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Try to get PR number from remaining args | ||
| if len(sys.argv) > idx + 2: | ||
| try: | ||
| pr_number = int(sys.argv[idx + 2]) | ||
| except ValueError: | ||
| pass |
There was a problem hiding this comment.
The argument parsing logic has an issue. When --body-file is used with a PR number (e.g., test_validation.py --body-file file.txt 12345), the PR number would be at index 3, but the code tries to parse it from sys.argv[idx + 2] where idx is the index of --body-file (index 1). This means sys.argv[3] would be the PR number, which is correct. However, if the command is test_validation.py 12345 --body-file file.txt, then idx is 2, and sys.argv[4] would be accessed, which might not exist. The logic should handle different argument orders more robustly.
| url = f"{base_url}?{query_string}" | ||
| url_ui = f"{base_url}?{query_string}&ui=true" | ||
|
|
||
| rows.append(f"| [}-4caf50?style=flat-square)]({url}) []({url_ui}) |") |
There was a problem hiding this comment.
The badge label for backport branches at line 192 uses branch.replace('-', '_') but doesn't handle commas. When a branch entry contains multiple branches separated by commas (e.g., "stable-25-2,stable-25-2-1,stable-25-3"), the commas will remain in the badge label, which may cause issues with URL encoding or badge rendering. Consider also replacing commas with underscores or another separator: branch.replace('-', '_').replace(',', '_').
| rows.append(f"| [}-4caf50?style=flat-square)]({url}) []({url_ui}) |") | |
| rows.append(f"| [.replace(',', '_')}-4caf50?style=flat-square)]({url}) []({url_ui}) |") |
| if not run_id: | ||
| raise ValueError("GITHUB_RUN_ID environment variable is not set") | ||
|
|
||
| return f"{github_server}/{github_repo}/actions/runs/{run_id}" |
There was a problem hiding this comment.
The PR comment logic identifies comments by workflow run URL (line 35), but the "Update PR comment with results" step runs within the matrix job (lines 245-256 in run_tests.yml). Since all matrix jobs share the same GITHUB_RUN_ID, they will all try to update the same comment. This will cause the last completing matrix job to overwrite the results from other jobs, losing information about tests for other branches. Consider either:
- Creating separate comments per matrix branch by including the branch in the identifier
- Moving the update step to a separate job that runs after all matrix jobs complete and aggregates results
- Including the matrix branch context in the comment identification and creating one comment per branch
This commit updates the issue pattern matching in `pr_template.py` to use a negative lookahead, preventing false matches with branch names in URLs. Additionally, the `validate_pr_description.py` script is modified to check for the "Bugfix" category in a case-insensitive manner, improving the accuracy of PR description validation. Key changes: - Updated issue pattern to avoid matching branch names. - Added case-insensitive check for "Bugfix" category in PR descriptions.
|
⚪
🟢 |
Co-authored-by: Copilot <[email protected]>
|
⚪
🟢 |
|
⚪
🟢 |
🔄 Backport
Legend:
|
|
|
Backported to |
There was a problem hiding this comment.
Мне кажется, что в .github/actions предполагаем видеть action.yaml и сопутствующую обвязку
Может это вынести в scripts?
|
|
||
| table = "<!-- test-execution-table -->\n" | ||
| table += "<h3>Run tests</h3>\n\n" | ||
| table += "| Small & Medium | Large |\n" |
There was a problem hiding this comment.
Почему бы не брать это из test_size_combinations?
| if not show_test_table and not show_backport_table: | ||
| return # No tables should be added | ||
|
|
||
| app_domain = os.environ.get("APP_DOMAIN") |
There was a problem hiding this comment.
Может сразу сделать normalize? Кажется, что сырое значение сейчас нигде не нужно
There was a problem hiding this comment.
И почему оно DOMAIN, а не какой-нибудь APP_HOST/APP_ENDPOINT?
There was a problem hiding this comment.
А в чем смысл этого файла?
| r"https://github.com/ydb-platform/[a-z\-]+/issues/\d+", | ||
| r"https://st.yandex-team.ru/[a-zA-Z]+-\d+", | ||
| r"#\d+", | ||
| r"[a-zA-Z]+-\d+" |
There was a problem hiding this comment.
Какую проблему это решило?
Что добавлено
Критерии приёмки
Changelog entry
...
Not for changelog (changelog entry is not required)
Changelog category
Description for reviewers
...
🔄 Backport
Legend: